fix(app): submit button not working in line-comment#18388
fix(app): submit button not working in line-comment#18388alikia2x wants to merge 1 commit intoanomalyco:devfrom
Conversation
|
The following comment was made by an LLM, it may be inaccurate: Based on my search, I found one potentially related PR: Related PR:
No other duplicate PRs were found specifically addressing the submit button issue in line-comment or the onDraftPopoverFocusOut callback removal. |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
This PR is indeed similar to #17520, but I think it provides a more thorough solution.
|
Issue for this PR
Type of change
What does this PR do?
onDraftPopoverFocusOutcallback chain inpackages/app/src/pages/session/file-tabs.tsxand related components (line-comment.tsxandline-comment-annotations.tsxinpackages/ui/src/components).To be short, this PR mainly addresses the unfinished work in PR #17948, which only resolved the issue in Review tab, but the problem still exists in the file tab.
Problem
When viewing a file in the file tab (the file viewer, not the dedicated Review tab), clicking the "Comment" (submit) button inside the inline comment editor popover does nothing. The only way to submit a comment is by pressing Enter.
Root Cause & Explanations (click for more details)
The
onDraftPopoverFocusOutcallback (file-tabs.tsx:220) was designed to close the draft when focus leaves the popover div.opencode/packages/app/src/pages/session/file-tabs.tsx
Lines 220 to 230 in 2c056c9
However, how the focus moves when user clicks differs across browsers and platforms. macOS's GUI habitually does not give the focus to the button when it's clicked. This means that in WebKit (which is also the WebView being used in OpenCode Desktop on macOS), clicking the comments button does not transfer focus to it. Instead, the entire popup loses focus. This causes
current.contains(document.activeElement)to be false, triggeringsetNote("commenting", null), leading to a bug.On Chromium, while focus does transfer to the button when clicked, a recent PR (#17948) added a
preventDefaultevent to the button's mousedown event. This means that even in Chromium-based browsers, clicking this button no longer transfers focus to it.Therefore, in the current situation, the
onDraftPopoverFocusOutfunction is purely a bug, hindering normal functionality without contributing anything positive.How did you verify your code works?
I built and ran the modified application and ensured that all functions worked correctly.
Screen recording below can be compared with recording within #18377 to demonstrate the proposed fix.
Explanation to potential concerns:
Even if
onDraftPopoverFocusOutis removed, users can still close the window by clicking on other lines, pressing Escape, or clicking the Cancel button.This change does not result in significant UI/UX changes or functional breakdown; on the contrary, it improves the UI/UX of the feature.
Screenshots / recordings
Recording: the submit button now works well
18377-fix-1.mp4
Screenshot: before this PR, the editor was too wide, making itself overflows.
Screenshot: if this PR gets merged, the comments editor will display normally both in review tab and file tab
Checklist